Use standard warnings for internal pytest warnings #3931

nicoddemus · 2018-09-04T14:49:52Z

This PR replaces our usage of the internal warnings system to use standard warnings instead.

Some comments:

New pytest_warning_capture hook.
Config.warn and Node.warn are now deprecated.
Terminal still captures the old pytest_logwarning hook to keep displaying warnings issued by Config.warn and Node.warn. When we remove Config.warn and Node.warn we are free to mark pytest_logwarning for deprecation.
Due to Warnings from conftest.py should show up in warning summary #2891, I needed to implement a small hack in order to show warnings during the configure stage.
Deprecation warnings are now shown by default (pytest should be overriding the interpreter's default warnings filter #2908).
Warnings are captured during collection (Warnings not captured during test collection #3251).

…nmanager It seems this has no effect since `pluggy` was developed as a separate library.

pytest_logwarning is no longer emitted by the warnings plugin, only ever emitted from .warn() functions in config and item

Fix pytest-dev#3251

Fix pytest-dev#2477

Fix pytest-dev#2452 Fix pytest-dev#2684

Fix pytest-dev#2908

Once we can capture warnings during the config stage, we can then get rid of this function Related to pytest-dev#2891

Standard warnings already contain the proper location, so we don't need to also print the node id

asottile

code looks good -- how does this interact with capwarn?

src/_pytest/assertion/rewrite.py

src/_pytest/cacheprovider.py

RonnyPfannschmidt · 2018-09-04T15:35:39Z

src/_pytest/cacheprovider.py


    @staticmethod
    def cache_dir_from_config(config):
        return paths.resolve_from_str(config.getini("cache_dir"), config.rootdir)

    def warn(self, fmt, **args):
-        self._warn(code="I9", message=fmt.format(**args) if args else fmt)
+        import warnings


lets please make the imports toplevel and introduce a PytestCacheWarning which expresses the warnings of this subsystem

@nicoddemus i beleive as a followup we should inline this method and adapt stacklevels, so people see the warnings located in their code

lets please make the imports toplevel and introduce a PytestCacheWarning which expresses the warnings of this subsystem

This is worth discussing a little: should we in general create a new warning class for each plugin, or just using PytestWarning is enough?

My take is that just having a few warning classes is enough, I don't see much reason to have a ton of warning classes unless we have a good use case for that.

I ask because there's only 2 calls to Config.warn:

self.warn("could not create cache path {path}", path=path) ... self.warn("cache could not write path {path}", path=path)

Is Config.warn something that we meant to be public so others use it? It feels like it was made public without much thought behind it, mostly seems like an accident to me actually.

@nicoddemus this self.warn can be completely replaced by a warnings.warn

i beleive we can remove it as internal api, if there turns out to be a user, we can bring it back

src/_pytest/config/__init__.py

src/_pytest/fixtures.py

RonnyPfannschmidt · 2018-09-04T15:44:33Z

src/_pytest/hookspec.py

+
+        * ``"config"``: during pytest configuration/initialization stage.
+        * ``"collect"``: during test collection.
+        * ``"runtest"``: during test execution.


for better tracking i propose having not just runtest, but also the item phases called setup, call, teardown

Good idea, done!

Oops just discovered that trying to catch warnings in pytest_runtest_setup, pytest_runtest_call and pytest_runtest_teardown cause problems with recwarn fixture. I don't see a way to fix this properly.

Any suggestions here @RonnyPfannschmidt ?

i believe we need to find a way to connect the fixture there - i propose postponing the detail work on that as i believe it will be tricky , require a tracking data-structure and a set of extra support utilities

OK, I'm reverting this to "runtest" then.

If we implement support for setup, call and teardown in the future, we will have a breaking change in our hands because of runtest...

actually, can we fake it by just implementing it like runtest, but switching the internal string based on phase?

But warnings are cached by warnings.catch_warnings, so it seems out of our control.

Feel free to try out an idea and pushing to my branch if you have a clear picture on how to implement it. 👍

RonnyPfannschmidt · 2018-09-04T19:05:12Z

src/_pytest/warnings.py

+        if item is not None:
+            for mark in item.iter_markers(name="filterwarnings"):
+                for arg in mark.args:
+                    warnings._setoption(arg)


btw - its a "bug" that we dont use our own setoption here, as it prevents regex usage

…ning_captured hook

nicoddemus · 2018-09-04T23:44:13Z

Finally all builds have passed. I believe I have addressed all comments, so this is ready for a final review. 👍

testing/test_mark.py

RonnyPfannschmidt

tremendous and fabulous work, this shaped up nicely over time

codecov · 2018-09-05T12:21:38Z

Codecov Report

Merging #3931 into features will increase coverage by 0.01%.
The diff coverage is 98.21%.

@@             Coverage Diff              @@
##           features    #3931      +/-   ##
============================================
+ Coverage     96.24%   96.25%   +0.01%     
============================================
  Files           108      107       -1     
  Lines         23419    23568     +149     
============================================
+ Hits          22539    22686     +147     
- Misses          880      882       +2

Flag	Coverage Δ
#doctesting	`31.78% <28.29%> (ø)`	⬆️
#nobyte	`93.53% <96.08%> (+0.01%)`	⬆️
#numpy	`31.31% <22.18%> (-0.06%)`	⬇️
#pexpect	`53.6% <51.39%> (+0.05%)`	⬆️
#pluggymaster	`95.1% <98.21%> (+0.04%)`	⬆️
#py27	`94.63% <98.21%> (+0.02%)`	⬆️
#py34	`93.92% <96.42%> (+0.04%)`	⬆️
#py35	`93.93% <96.42%> (+0.04%)`	⬆️
#py36	`94.85% <96.42%> (+0.02%)`	⬆️
#py37	`94.09% <96.42%> (+0.04%)`	⬆️
#trial	`33.95% <22.18%> (-0.08%)`	⬇️
#xdist	`94.67% <97.91%> (+0.02%)`	⬆️

Impacted Files	Coverage Δ
testing/test_warnings.py	`99.15% <100%> (+0.71%)`	⬆️
testing/test_terminal.py	`99.4% <100%> (ø)`	⬆️
src/_pytest/warning_types.py	`100% <100%> (ø)`
src/_pytest/deprecated.py	`100% <100%> (ø)`	⬆️
testing/test_resultlog.py	`100% <100%> (ø)`	⬆️
testing/test_capture.py	`99.24% <100%> (-0.01%)`	⬇️
src/_pytest/pytester.py	`89.05% <100%> (-0.02%)`	⬇️
src/_pytest/fixtures.py	`98.49% <100%> (ø)`	⬆️
testing/test_junitxml.py	`98.57% <100%> (ø)`	⬆️
testing/python/metafunc.py	`95.74% <100%> (-0.09%)`	⬇️
... and 23 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 29bfa5e...f63c683. Read the comment docs.

coveralls · 2018-09-05T13:04:38Z

Coverage increased (+0.05%) to 94.088% when pulling f63c683 on nicoddemus:internal-warnings into 29bfa5e on pytest-dev:features.

Fix pytest-dev#3936

RonnyPfannschmidt · 2018-09-05T13:43:07Z

@nicoddemus @blueyed it seems quite painful that the baseline now takes more time that a full test originally

blueyed · 2018-09-05T19:54:57Z

@RonnyPfannschmidt

it seems quite painful that the baseline now takes more time that a full test originally

?
Do you mean the 4 test jobs are slower than the full build?
Please provide a reference then (URL, Travis/AV?).. the full build was ~2h before, so I do not think that's the case really.

nicoddemus added 22 commits September 4, 2018 11:35

Remove assert for "reprec" because this is no longer set on the plugi…

10f21b4

…nmanager It seems this has no effect since `pluggy` was developed as a separate library.

Implement new pytest_warning_captured hook

ffd47ce

Make terminal capture pytest_warning_capture

3fcc4cd

pytest_logwarning is no longer emitted by the warnings plugin, only ever emitted from .warn() functions in config and item

Remove Python 2.6 specific warning

51e32cf

Capture and display warnings during collection

1a9d913

Fix pytest-dev#3251

Start the laywork to capture standard warnings

0100f61

Use std_warn for warning about applying marks directly to parameters

8e4501e

Fix existing tests now that we are using standard warnings

0c8dbdc

Deprecate Config.warn and Node.warn, replaced by standard warnings

78ac7d9

Make PytestWarning and RemovedInPytest4Warning part of the public API

19a01c9

Add docs for internal warnings and introduce PytestDeprecationWarning

208dd3a

Fix pytest-dev#2477

Add CHANGELOG entries for pytest-dev#2452

7e13593

Fix pytest-dev#2452 Fix pytest-dev#2684

Show deprecation warnings by default if no other filters are configured

9965ed8

Fix pytest-dev#2908

Add test to ensure that users can suppress internal warnings

60499d2

Implement hack to issue warnings during config

0fffa6b

Once we can capture warnings during the config stage, we can then get rid of this function Related to pytest-dev#2891

Remove nodeid from messages for warnings generated by standard warnings

56d4141

Standard warnings already contain the proper location, so we don't need to also print the node id

Improve docs for warnings capture and PEP-0506 remarks

b818314

Move PytestExerimentalApiWarning to warning_types

8ce3aea

Fix typo in PytestExperimentalApiWarning

415a62e

Remove commented out code

c304998

Add comment about deprecation warnings being shown by default

e9417be

Improve get_fslocation_from_item's docstring

016f8f1

nicoddemus changed the title ~~Internal warnings~~ Use standard warnings for internal pytest warnings Sep 4, 2018

nicoddemus requested review from RonnyPfannschmidt and asottile September 4, 2018 14:50

asottile approved these changes Sep 4, 2018

View reviewed changes

src/_pytest/assertion/rewrite.py Outdated Show resolved Hide resolved

src/_pytest/assertion/rewrite.py Outdated Show resolved Hide resolved

src/_pytest/cacheprovider.py Outdated Show resolved Hide resolved

RonnyPfannschmidt reviewed Sep 4, 2018

View reviewed changes

src/_pytest/config/__init__.py Outdated Show resolved Hide resolved

RonnyPfannschmidt reviewed Sep 4, 2018

View reviewed changes

src/_pytest/fixtures.py Outdated Show resolved Hide resolved

RonnyPfannschmidt reviewed Sep 4, 2018

View reviewed changes

nicoddemus added 8 commits September 4, 2018 16:29

Use explicit instances when calling warnings.warn_explicit

d3ca739

Keep backward compatibility for code as kw in Node.warn

b7560a8

Fix stacklevel for warning about Metafunc.addcall

6d497f2

Make config no longer optional in parametrize id functions

5a52aca

Revert to having just "runtest" as "when" parameter of the pytest_war…

2e0a7cf

…ning_captured hook

Improve test_rewarn_functional

4592def

Fix test_idval_hypothesis

adc9ed8

Make code_or_warning parameter private for backward-compatibility

f42b501

RonnyPfannschmidt reviewed Sep 5, 2018

View reviewed changes

testing/test_mark.py Outdated Show resolved Hide resolved

RonnyPfannschmidt approved these changes Sep 5, 2018

View reviewed changes

auvipy approved these changes Sep 5, 2018

View reviewed changes

Make sure warn is called in test_parameterset_extractfrom

ddb3084

RonnyPfannschmidt approved these changes Sep 5, 2018

View reviewed changes

No longer escape regex in pytest.mark.filterwarnings

f63c683

Fix pytest-dev#3936

nicoddemus merged commit 531b76a into pytest-dev:features Sep 5, 2018

nicoddemus deleted the internal-warnings branch September 5, 2018 17:05

nicoddemus mentioned this pull request Sep 5, 2018

Timeline for removal of deprecated astropy.tests.pytest_plugins astropy/astropy#7782

Closed

vbarbaresi mentioned this pull request Oct 15, 2018

Add -rW option to print captured warnings even for passing tests #3633

Closed

The-Compiler mentioned this pull request Aug 18, 2020

Warnings from conftest.py should show up in warning summary #2891

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Use standard warnings for internal pytest warnings #3931

Use standard warnings for internal pytest warnings #3931

nicoddemus commented Sep 4, 2018

asottile left a comment

RonnyPfannschmidt Sep 4, 2018

RonnyPfannschmidt Sep 4, 2018

nicoddemus Sep 4, 2018

RonnyPfannschmidt Sep 4, 2018

RonnyPfannschmidt Sep 4, 2018

nicoddemus Sep 4, 2018

nicoddemus Sep 4, 2018

RonnyPfannschmidt Sep 4, 2018

nicoddemus Sep 4, 2018

RonnyPfannschmidt Sep 4, 2018

nicoddemus Sep 4, 2018

RonnyPfannschmidt Sep 4, 2018

nicoddemus commented Sep 4, 2018 •

edited

Loading

RonnyPfannschmidt left a comment

codecov bot commented Sep 5, 2018 •

edited

Loading

coveralls commented Sep 5, 2018 •

edited

Loading

RonnyPfannschmidt commented Sep 5, 2018

blueyed commented Sep 5, 2018 •

edited

Loading

Use standard warnings for internal pytest warnings #3931

Use standard warnings for internal pytest warnings #3931

Conversation

nicoddemus commented Sep 4, 2018

asottile left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nicoddemus commented Sep 4, 2018 • edited Loading

RonnyPfannschmidt left a comment

Choose a reason for hiding this comment

codecov bot commented Sep 5, 2018 • edited Loading

Codecov Report

coveralls commented Sep 5, 2018 • edited Loading

RonnyPfannschmidt commented Sep 5, 2018

blueyed commented Sep 5, 2018 • edited Loading

nicoddemus commented Sep 4, 2018 •

edited

Loading

codecov bot commented Sep 5, 2018 •

edited

Loading

coveralls commented Sep 5, 2018 •

edited

Loading

blueyed commented Sep 5, 2018 •

edited

Loading